Conversation
|
@huseeiin is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughNon-lazy route handlers in the routing template are now imported dynamically with top-level Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/build/virtual/routing.ts`:
- Around line 33-36: The generated import line currently swallows all import
errors via .catch(() => {}) and then destructures default into ${h._importHash},
which hides whether the import failed or returned no default; change the
generated code so you import into a module variable (e.g., const mod = await
import("${h.handler}").catch((e) => { throw new Error(`Failed to import handler
${h.handler}: ${e?.message||e}`); });), then explicitly validate that mod &&
mod.default exists before assigning to ${h._importHash} and if not throw a clear
error mentioning the handler path and import hash (use h.handler and
h._importHash) so failures surface with actionable context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6fe85968-df96-444c-9193-6e48a7175de6
📒 Files selected for processing (1)
src/build/virtual/routing.ts
| .map( | ||
| (h) => | ||
| /* js */ `const { default: ${h._importHash} } = await import("${h.handler}").catch(() => {})` | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node --input-type=module <<'EOF'
const noDefault = await import('data:text/javascript,export const handler = () => {}');
console.log('default in namespace:', 'default' in noDefault, 'value:', noDefault.default);
try {
const { default: handler } = await import('data:text/javascript,throw new Error("boom")').catch(() => {});
console.log('handler:', handler);
} catch (error) {
console.error('error after catch:', error.message);
}
EOFRepository: nitrojs/nitro
Length of output: 221
🏁 Script executed:
cd src/build/virtual && [ -f routing.ts ] && cat -n routing.ts | head -50Repository: nitrojs/nitro
Length of output: 2158
🏁 Script executed:
rg "\.catch\(\(\)" src/build/virtual/routing.ts -A 2 -B 2Repository: nitrojs/nitro
Length of output: 194
🏁 Script executed:
git ls-files src/build/virtual/Repository: nitrojs/nitro
Length of output: 506
🏁 Script executed:
rg "\.catch\s*\(\s*\(\s*\)\s*=>\s*\{\s*\}" src/build --type tsRepository: nitrojs/nitro
Length of output: 185
🏁 Script executed:
rg "await import.*catch" src/build --type ts -A 1 -B 1Repository: nitrojs/nitro
Length of output: 258
🏁 Script executed:
rg "handler.*import" src/build/virtual/routing.ts -A 2 -B 2Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
git log --oneline src/build/virtual/routing.ts | head -5Repository: nitrojs/nitro
Length of output: 83
🏁 Script executed:
git show 1b13f32 --statRepository: nitrojs/nitro
Length of output: 50369
🏁 Script executed:
git show 1b13f32 -- src/build/virtual/routing.ts | head -80Repository: nitrojs/nitro
Length of output: 2734
Don't silently swallow handler import failures with .catch(() => {}).
Line 35 suppresses import errors and leaves the destructuring to fail later with an opaque message. When import() succeeds but has no default export, the destructuring still fails downstream without context about which handler path failed. This violates the coding guideline to prefer explicit errors and include actionable context.
Validate mod.default explicitly after import and throw with the handler path included:
Proposed fix
${allHandlers
.filter((h) => !h.lazy)
.map(
- (h) =>
- /* js */ `const { default: ${h._importHash} } = await import("${h.handler}").catch(() => {})`
+ (h) =>
+ /* js */ `
+const ${h._importHash}Module = await import("${h.handler}").catch((error) => {
+ throw new Error(${JSON.stringify(`Failed to import route handler "${h.handler}"`)} + ": " + String(error));
+});
+if (${h._importHash}Module.default === undefined) {
+ throw new Error(${JSON.stringify(`Route handler "${h.handler}" must have a default export`)});
+}
+const ${h._importHash} = ${h._importHash}Module.default;`
)
.join("\n")}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .map( | |
| (h) => | |
| /* js */ `const { default: ${h._importHash} } = await import("${h.handler}").catch(() => {})` | |
| ) | |
| .map( | |
| (h) => | |
| /* js */ ` | |
| const ${h._importHash}Module = await import("${h.handler}").catch((error) => { | |
| throw new Error(${JSON.stringify(`Failed to import route handler "${h.handler}"`)} + ": " + String(error)); | |
| }); | |
| if (${h._importHash}Module.default === undefined) { | |
| throw new Error(${JSON.stringify(`Route handler "${h.handler}" must have a default export`)}); | |
| } | |
| const ${h._importHash} = ${h._importHash}Module.default;` | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/build/virtual/routing.ts` around lines 33 - 36, The generated import line
currently swallows all import errors via .catch(() => {}) and then destructures
default into ${h._importHash}, which hides whether the import failed or returned
no default; change the generated code so you import into a module variable
(e.g., const mod = await import("${h.handler}").catch((e) => { throw new
Error(`Failed to import handler ${h.handler}: ${e?.message||e}`); });), then
explicitly validate that mod && mod.default exists before assigning to
${h._importHash} and if not throw a clear error mentioning the handler path and
import hash (use h.handler and h._importHash) so failures surface with
actionable context.
|
@pi0 coderabbitai is right, this ignores all errors. what should we do? |
1 participant
in dev nitro doesn't complain if there is no default export in
server.ts, this fixes it in prod.🔗 Linked issue
❓ Type of change
📚 Description
📝 Checklist